[WIP] Branch for 0.2.0 #84

parro-it · 2018-04-10T18:47:35Z

Well, I made a mess by mistakenly merging the binding branch on master instead 0_2_0.
So, this a new PR for 0_2_0 version. Master now contains area-fixes branch and what not...

Still now, we have:

Am I forgotting somethings?

…p integration

…_2_0

parro-it · 2018-04-24T14:57:30Z

The problem disappear if I move the stream to top level, and close it before closing the window...

But it's not related to node stream. If you set an interval sufficiently low for setInterval, the same thing happen.

Meanwhile, I noticed there's a new released version of npm that doesn't support node 4 anymore, so I lowered the version used on AppVeyor to 5.7

mischnic · 2018-04-25T14:50:19Z

Did you have that error on Windows as well?

$ git commit -m "Really fix eventloop example"
husky > npm run -s precommit (node v9.7.1)

git pushError running git-clang-format: { Error: spawnSync C:\Users\nikla\Desktop\DEV\libui-node\node_modules\clang-format\bin\git-clang-format ENOENT
    at Object.spawnSync (internal/child_process.js:956:20)
    at spawnSync (child_process.js:561:24)
    at main (C:\Users\nikla\Desktop\DEV\libui-node\node_modules\clang-format\bin\check-clang-format.js:64:18)
    at Object.<anonymous> (C:\Users\nikla\Desktop\DEV\libui-node\node_modules\clang-format\bin\check-clang-format.js:83:22)
    at Module._compile (module.js:662:30)
    at Object.Module._extensions..js (module.js:673:10)
    at Module.load (module.js:575:32)
    at tryModuleLoad (module.js:515:12)
    at Function.Module._load (module.js:507:3)
    at Function.Module.runMain (module.js:703:10)
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawnSync C:\\Users\\nikla\\Desktop\\DEV\\libui-node\\node_modules\\clang-format\\bin\\git-clang-format',
  path: 'C:\\Users\\nikla\\Desktop\\DEV\\libui-node\\node_modules\\clang-format\\bin\\git-clang-format',
  spawnargs: [ '--diff' ] }


husky > pre-commit hook failed (add --no-verify to bypass)

parro-it · 2018-04-25T18:46:09Z

examples/event-loop.js

@@ -57,7 +57,7 @@ libui.startLoop();
 const linebreak = process.platform === "win32" ? '\r\n' : '\n';


We can use require('path').sep to get the correct line break for the platform.

Yes, but actually, it's require('os').EOL.
require('path').sep is slash or backslash for filepaths.

It's also interesting that textarea.getText() returns the text with "\n" (also on windows), but setText() requires the platform specific type.

Oh yes, sorry...

It's also interesting that textarea.getText() returns the text with "\n" (also on windows), but setText() requires the platform specific type.

That's strange, we should investigate if it's intentional or not.

parro-it · 2018-04-25T19:19:37Z

Did you have that error on Windows as well?

Err, I didn't yet configure clang-format on Windows, I just skip the check 🤨

mischnic · 2018-04-28T19:59:14Z

event-loop.js with Node 10: Started the server and did curl localhost:8300

$ node event-loop.js
2018-04-28 21:57:20.839 node[6829:2371853] get 0x0
******** listeining...
net.js:416
  clearTimeout(this[kTimeout]);
  ^

TypeError: Type mismatch
    at Socket.setTimeout (net.js:416:3)
    at connectionListenerInternal (_http_server.js:338:12)
    at defaultTriggerAsyncIdScope (internal/async_hooks.js:294:19)
    at Server.connectionListener (_http_server.js:319:3)
    at Server.emit (events.js:182:13)
    at TCP.onconnection (net.js:1550:8)

mischnic · 2018-04-29T15:25:33Z

event-loop.js with Node 10: Started the server and did curl localhost:8300

This is somehow fixed in the uitimer branch (which uses the libui timer, #92).

parro-it · 2018-04-29T15:53:58Z

It seems they are using global clearTimeout (that we patched) to cancel a timer not create by our version of setTimeout:

https://github.com/nodejs/node/blob/9c48926dba20a507968d4152fafa9d51d69cf970/lib/net.js#L410-L430

We should check other similar usage in node source code to prevent other bugs...

parro-it · 2018-04-29T15:57:35Z

The event loop example crashed once when closing the window on Sierra, but now it works again...

I thought of a solution for some days, but still now I can think of a valid one. So I'll just for now comment the lines that clean up events callbacks, to avoid the segmentation fault.

This cause some memory to leak on every registered callback, but so we can go ahead with 0_2_0 release and work in parallel to other features

parro-it · 2018-04-29T17:19:32Z

This is somehow fixed in the uitimer branch (which uses the libui timer, #92).

The TypeError there is gone because clearTimeout is implemented in JavaScript without checking the type of the argument. And in that function, you just set a property on the argument, so it work with whatever argument type is passed:

libui-node/index.js

Lines 66 to 68 in 136856c

    
           global.clearTimeout = function(timeoutHandle) { 
        
           	timeoutHandle.running = false; 
        
           }

But I guess that the clearTimeout in node net.js that throws before is still not clearing the timeout as it should. We should also patch whatever function they used to create that timer

…linux

parro-it · 2018-04-30T07:03:05Z

@mischnic, can you confirm the last commit solve the callback problem on Sierra? My VM stop working for some obscure reason 😢
You should still get a dump, similar to the one below. This is due to the fact that the UiArea is accessed after it has been destroyed, and will be fixed by #75 .

0   CoreFoundation                      0x00007fff2c82c6bb __exceptionPreprocess + 171
	1   libobjc.A.dylib                     0x00007fff53f44942 objc_exception_throw + 48
	2   CoreFoundation                      0x00007fff2c8c39e4 -[NSObject(NSObject) doesNotRecognizeSelector:] + 132
	3   CoreFoundation                      0x00007fff2c7a4183 ___forwarding___ + 1443
	4   CoreFoundation                      0x00007fff2c7a3b58 _CF_forwarding_prep_0 + 120
	5   libui.A.dylib                       0x0000000102cb0803 uiMultilineEntryText + 35
	6   nbind.node                          0x00000001029d835f _ZN16UiMultilineEntry7getTextEv + 33
	7   nbind.node                          0x00000001029d9a28 _ZN5nbind6CallerINSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEENS_8TypeListIJEEEE10callMethodI16UiMultilineEntryMSC_FS7_vEKN3Nan20PropertyCallbackInfoIN2v85ValueEEEEENSH_5LocalISI_EERT_T0_RT1_ + 60
	8   nbind.node                          0x00000001029d9951 _ZN5nbind22TemplatedBaseSignatureINS_15GetterSignatureIM16UiMultilineEntryFNSt3__112basic_stringIcNS3_11char_traitsIcEENS3_9allocatorIcEEEEvES2_NS_14PolicyListTypeIJEEES9_JEEESD_S9_JEE15callInnerSafelyIS2_KN3Nan20PropertyCallbackInfoIN2v85ValueEEESM_EEvRT0_RT1_j + 119
	9   nbind.node                          0x0000000102a37ff8 _ZN3Nan3impL21GetterCallbackWrapperEN2v85LocalINS1_4NameEEERKNS1_20PropertyCallbackInfoINS1_5ValueEEE + 139
	10  ???                                 0x000003fe0d5b5de0 0x0 + 4389680668128

mischnic · 2018-04-30T07:41:55Z

Yes, now I'm get something regarding UiMultilineEntry::getText().

parro-it · 2018-04-30T07:44:04Z

Yes, now I'm get something regarding UiMultilineEntry::getText().

Fine, thank you, that will be fixed in 0_3_0. So I'll go ahead and publish a pre-release version on npm.

But first I have to bump some dep version on libui-download in order to solve an issue warning regarding hoek package.

mischnic · 2018-04-30T07:44:44Z

What about the clearTimeout problem?

parro-it · 2018-04-30T07:51:25Z

What about the setTimeout problem?

I plan to solve that too in 0_3_0.

My last thought (hopes) on regards are that by limiting the time we wait for GUI events in main thread, timers works without patching them.

Otherwise, we'll probably need to patch internal node Timeout class, that is the base for all the various timers

parro-it · 2018-04-30T08:01:32Z

Hey it seems you solved it? This imply that node is passing a null timeout handle to clear?

mischnic · 2018-04-30T08:03:15Z

This is one half of the fix, the Timeout object still get's through and it still crashes.

…interval is created by us. If not, calls the original node functions.

parro-it · 2018-04-30T08:48:26Z

With my last commit, we are now checking if the timeout given as argument is an instance of our TimeoutHandle class. If not, it calls the original clearTimeout or clearInterval functions.

The crash now does not happens, but there is still some problem to solve, because the timers that node use internally still expires only if there are GUI events (since they are not pached)

parro-it and others added 30 commits March 20, 2018 23:15

Attempt to improve (fix) Windows event loop

3f79cd1

core api example: add interval and http server to test node event loo…

0c9006d

…p integration

Fixed main GUI loop wake up

a5253e4

logs & tweaks

4d86348

fix & some comments

a669c4f

loop improvements on linux

73dd891

Merge branch 'macos_event_loop' into windows_event_loop

4eed63c

basic timer support

531149d

Reduce timeout in background event thread to a max of 100ms

30aff63

Extracted event loop includes for commmon & linux

cbf3fdb

DEBUG macro for logging

c87b9fb

external include for darwin

398d55b

lint

aa8c782

Add usleep implementation for windows

55fbf09

moved check for stopping non running loop

cd0a93a

typo

72acc7c

merge windows loop improvements

5572a78

Fix stopLoop

3534ede

moved uv struct to header

888803b

Fix loop on unix

40064a2

Fix loop_t redeclaration on windows

73b4997

Fix loop on windows

c12a913

Reverted event loop stop change

a1a70e8

merged master

d69d0a0

Font header

e88d1c1

Work

d8e3db8

Fixup

df26d26

Compile errors

957a61d

Attribute, AttributedString, fixes

c5c0534

Update examples, fix almost everything

8e29ebe

parro-it added 2 commits April 24, 2018 16:45

Install older npm version, since latest does not run anymore on node 4

232d9db

Merge branch '0_2_0' of https://github.com/parro-it/libui-node into 0…

80a2488

…_2_0

Really fix eventloop example

6386588

parro-it commented Apr 25, 2018

View reviewed changes

Fix examples

f3dd4eb

mischnic mentioned this pull request Apr 28, 2018

TextArea.getText() line ending discrepancy on Windows #97

Closed

parro-it added 2 commits April 29, 2018 18:28

update pkg lock

443a22a

commented DISPOSE_EVENT macro

c581aba

multiline comments with // cause warnings when compiling with gcc on …

feaedf0

…linux

parro-it and others added 2 commits April 30, 2018 09:54

bump libui-download version to get rid of security issues

46291ff

Handle null with clearTimeout/Interval

998adc4

Further mitigation for clear timers problem. Check if the timeout or …

ab47e03

…interval is created by us. If not, calls the original node functions.

Add node 10 to CI

e634711

parro-it merged commit 66885e7 into master May 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Branch for 0.2.0 #84

[WIP] Branch for 0.2.0 #84

parro-it commented Apr 10, 2018 •

edited

Loading

parro-it commented Apr 24, 2018

mischnic commented Apr 25, 2018

parro-it Apr 25, 2018

mischnic Apr 25, 2018 •

edited

Loading

parro-it Apr 25, 2018

parro-it commented Apr 25, 2018

mischnic commented Apr 28, 2018 •

edited

Loading

mischnic commented Apr 29, 2018

parro-it commented Apr 29, 2018

parro-it commented Apr 29, 2018

parro-it commented Apr 29, 2018

parro-it commented Apr 30, 2018

mischnic commented Apr 30, 2018

parro-it commented Apr 30, 2018

mischnic commented Apr 30, 2018 •

edited

Loading

parro-it commented Apr 30, 2018

parro-it commented Apr 30, 2018

mischnic commented Apr 30, 2018

parro-it commented Apr 30, 2018

		@@ -57,7 +57,7 @@ libui.startLoop();
		const linebreak = process.platform === "win32" ? '\r\n' : '\n';

[WIP] Branch for 0.2.0 #84

[WIP] Branch for 0.2.0 #84

Conversation

parro-it commented Apr 10, 2018 • edited Loading

parro-it commented Apr 24, 2018

mischnic commented Apr 25, 2018

parro-it Apr 25, 2018

Choose a reason for hiding this comment

mischnic Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

parro-it Apr 25, 2018

Choose a reason for hiding this comment

parro-it commented Apr 25, 2018

mischnic commented Apr 28, 2018 • edited Loading

mischnic commented Apr 29, 2018

parro-it commented Apr 29, 2018

parro-it commented Apr 29, 2018

parro-it commented Apr 29, 2018

parro-it commented Apr 30, 2018

mischnic commented Apr 30, 2018

parro-it commented Apr 30, 2018

mischnic commented Apr 30, 2018 • edited Loading

parro-it commented Apr 30, 2018

parro-it commented Apr 30, 2018

mischnic commented Apr 30, 2018

parro-it commented Apr 30, 2018

parro-it commented Apr 10, 2018 •

edited

Loading

mischnic Apr 25, 2018 •

edited

Loading

mischnic commented Apr 28, 2018 •

edited

Loading

mischnic commented Apr 30, 2018 •

edited

Loading